Skip to content

Conversation

@prafull01
Copy link
Collaborator

No description provided.

@prafull01 prafull01 force-pushed the remove-deprecated-fields branch 3 times, most recently from 13ac257 to b01e115 Compare September 25, 2025 11:22
@prafull01 prafull01 requested review from NishanthNalluri and pritesh-lahoti and removed request for pritesh-lahoti September 25, 2025 12:29
clusterSettings: ~
# timestamp captures the annotation timestamp used for rolling restarts.
timestamp: "2021-10-18T00:00:00Z"
# resources captures the resource requests and limits for CockroachDB pods.
Copy link
Contributor

@pritesh-lahoti pritesh-lahoti Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we need to remove the whole resources section here.

@prafull01 prafull01 force-pushed the remove-deprecated-fields branch 2 times, most recently from 0f4e15d to 1238344 Compare September 29, 2025 14:29
@prafull01 prafull01 force-pushed the remove-deprecated-fields branch 3 times, most recently from 1238344 to f886c28 Compare September 30, 2025 04:50
# If specified, podTemplate is merged with the default pod specification, with settings in podTemplate taking precedence.
# This can be used to add or update containers, volumes, and other settings of the CockroachDB pod.
podTemplate: {}
podTemplate:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: need to fix the indentation (of comments) within the podTemplate section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@pritesh-lahoti pritesh-lahoti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prafull01 - as discussed, let's add a changelog entry for these changes and seek @jhlodin 's review for it.

@prafull01 prafull01 force-pushed the remove-deprecated-fields branch from 647a3a0 to 3ab1dae Compare October 28, 2025 19:33
@prafull01 prafull01 requested review from jhlodin and removed request for jhlodin October 28, 2025 19:34
CHANGELOG.md Outdated
All notable changes to this project will be documented in this file.

## [cockroachdb-parent-25.3.3-preview+1] 2025-10-29
- Remove the deprecated fields and start using the podTemplate fields.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should list the deprecated fields, at least at a high level. Like:

- Removed the following deprecated fields in favor of `podTemplate`:
    - `crdbCluster.resources.*`
    - `crdbCluster.podLabels.*`
    - `crdbCluster.topologySpreadConstraints.*`

etc.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll also make it easier for me to identify which parts of the docs need to be updated to remove reference to deprecated, so if for some reason we don't want to list this out in the release note I could still use that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand to know before hand which fields are deprecated. However this change would make the CHANGELOG.md very log. Should we add that description here or I can add that in the ticket so it would be easier for you to write the documentation. This will add additional 10-12 line specific to deprecation in CHANGELOG.

cc @pritesh-lahoti

Copy link

@jhlodin jhlodin Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think 10-12 lines is acceptable. I've put similar lists in breaking changes/release notes in the past and AFAIK it was received well - people appreciate having the detail when you introduce a breaking change like deprecation/removal, and if they need to ctrl+F a field it's useful having it in the list. Best if we can have sub-bullets for each field like I proposed in my first comment, rather than individual release notes for every deprecation.

@prafull01 prafull01 force-pushed the remove-deprecated-fields branch from 3ab1dae to e0b0875 Compare October 30, 2025 14:30
@prafull01 prafull01 requested a review from jhlodin October 30, 2025 19:21
@prafull01 prafull01 force-pushed the remove-deprecated-fields branch 2 times, most recently from 1c7e18c to 7403e57 Compare October 30, 2025 19:31
Copy link

@jhlodin jhlodin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One quick suggestion, otherwise LGTM

CHANGELOG.md Outdated
All notable changes to this project will be documented in this file.

## [cockroachdb-parent-25.3.3-preview+1] 2025-10-29
- Remove the deprecated fields and start using the podTemplate fields. Following fields are deprecated:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Remove the deprecated fields and start using the podTemplate fields. Following fields are deprecated:
- Remove the following deprecated fields in favor of the corresponding podTemplate fields:

@prafull01 prafull01 force-pushed the remove-deprecated-fields branch from 7403e57 to e2c081c Compare October 31, 2025 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants